Add integration tests checking the physical observables resulting from different the integrators#521
Add integration tests checking the physical observables resulting from different the integrators#521thomasloux wants to merge 27 commits intoTorchSim:mainfrom
Conversation
- change default values corresponding to args used in tests with LJ argon - correct severe errors in equations (npt_langevin, npt_anisotropic_crescale)
…masloux/torch-sim into feat/add-physical-validation
In the previous commit:
What's next?Regarding integrators
Docs
|
…3 strains for x,y,z independant control
… the default parameters)
…t than device(cuda) (which I believe is not a problem)
…ror on unwrapping npt simulation
|
Ok I've managed to make all integrator pass: Changelog:
Others:
Note:
|
| [project.optional-dependencies] | ||
| test = [ | ||
| "torch-sim-atomistic[io,symmetry,vesin]", | ||
| "physical-validation>=1.0.5", |
There was a problem hiding this comment.
nit: This package seems unsupported
There was a problem hiding this comment.
I believe that's the last version on pypi and it's working on my computer. What is supposed to be unsupported?
| # Map masses to have batch dimension | ||
| M_2 = 2 * state.masses.unsqueeze(-1) # shape: (n_atoms, 1) | ||
|
|
||
| # Calculate new cell length scale (cube root of volume for isotropic scaling) |
There was a problem hiding this comment.
the new beta coming in externalizes this to another method correct?
There was a problem hiding this comment.
That's a bit weird but actually the random noise beta (both for cell or positions) is supposed to be the same one in equation of position and velocities: _npt_langevin_cell_position_step and _npt_langevin_cell_velocity_step for instance.
| # ============================================================================= | ||
| # NPT Langevin Strain integrator — isotropic logarithmic strain coordinate | ||
| # ============================================================================= |
There was a problem hiding this comment.
nit: can we remove CC headers and titles? Things like titles if used we need to keep consistent and so easier just not to have them imo
Signed-off-by: Rhys Goodall <rhys.goodall@outlook.com>
|
By the way, how do you want to handle the integrator names regarding whether they provide isotropic or anisotropic (length only or length+angle) ? NPT Crescale has these 3, now NPT Langevin supports isotropic and independant length. |
|
By the way, I need to correct the pytest tests. I was lazy running them, but now it only takes 15 minutes on a gpu, so that's fairly short. I'll add a doc with my jupyter notebook to show the plots. Note: there still pass on the notebook I use to make the tests. |
…masloux/torch-sim into feat/add-physical-validation
…opic or triclinic cell fluctuation
…hey are not core files
Summary
I've added tests to evaluate the correctness of integrators. I'm using physical_validation, used in gromacs test suite. The code is WIP, so that people can try it easily and update some parameters. In the future, the systems studied and the parameters will need to be fixed, or used the standard parameters. Note: this is important because the default parameters may be changed in an undesirable way.
What to do next?
Notes